-
Notifications
You must be signed in to change notification settings - Fork 563
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Trilinos Master Merge PR Generator: Auto PR created to promote from master_merge_20190124_010614 branch to master #4255
Conversation
The directory handles std::vectors of a type and can have variable length. An add mode is also implemented. The current unit test creates std::vector<int> of variable length for the user_t, fills the elements with count 1, then calls add which sums the arrays to provide a count of total processes sharing the same gid. The test validates the find call obtains the correct value by checking one of the array elements (all currently the same). reverse execution works for variable length without prior knowledge of the sizes. They are collected from the process which stores the data for that node. The other 3 tests (Original,Relic,Tpetra) will pass when the test is reconfigured for non-array mode without using resize. These will be developed and used for performance comparisons and eventually deleted. The remove feature was previously working but is temporarily turned off - will be restored for future commits. There are a several places which calculate offsets in a slow manual way to be refactored or stored in arrays later, but that will be inefficient in the present form. There are also many places which need further cleaning up to make the style more C++ like. Especially the handling of ptrs for the message types.
Fixes a bunch of type mismatches I wasn't properly checking for earlier. Fixes a few errors for the various modes and removes some unneeded code. Upgrades the tests to both an Add and Replace check. Started setup for an Aggregate mode as well but not implemented. Adds some debugging tools to the communicator class. Also added more careful comments for execute_wait. Next step will be delve into this more carefully - I have this working for variable sizes reverse execution but possibly this can be done in a simpler way. The relevant code is all in execute_wait and once I understand it better I expect it can be adapted to the current code. I am not sure if the original directory is already set up to handle this case or if we need some new code like this to make it work. There is a bug for small total ID count (such as 10) which is caused by no_send_buff becoming 1 in constructor for some cases due to the gid list being simple and happening to have all gids sorted by process. The bug hits in execute_wait so I am fairly certain this is all related to the same issues. Other tasks to also be done: Implement Aggregate Check/implement gid of multiple components - such as int[4]. Check edges cases more carefully (for example set to rec 0 items) Setup a PR So this is a checkpoint and work in progress. Note all 4 modes should pass unit tests (Kokkos, Original, Relic, Tpetra) but only Kokkos is currently set up to be using the new variable array length mode. I may fix Relic to do as well - Tpetra and Original won't ever support that anyways.
This removes some unnecessary exchanges and makes the directory work with variable sizes in a manner which I think is much closer to the original intention. The communicator do_wait function has been modified to handle the variable size. That part will need further discussion. I have also modified the resize method to handle the case which was previously marked as unsupported. Even though this is working for Add and Replace with variable length arrays, I'm not confident this is the best way to organize things and will need further discussion. Note searching for NEW METHOD will show the 5 places I think I need to discuss. This commit fixes a bunch of other issues with the directory.
This adds a working aggregate mode to the Kokkos directory. Also cleans up some issues with the test organization. Aggregate mode will combine the update arrays preserving only unique values and also preserving ordering. For example: proc 1 updates gid 10 with array values: 5, 6, 14 proc 2 updates gid 10 with array values: 6, 8, 20 A find on gid 10 will then produce array values: 5, 6, 8, 14, 20
Testing infrastructure developed to be more thorough. The unit test now grinds through many combinations of total Ids, vector/single user_t, different gid_t, gid_t as a single value or set of values, and also tests Add, Replace, and Aggregate. Note that only Kokkos mode tests all of Add, Replace, Aggregate along with vector user_t. Other modes are for development and eventually will go away. Added a simple test directoryTest_KokkosSimple for easier example of using the directory. The main test is thorough but getting opaque so this is intended as a simple example of how to setup the directory and use it. Added testing for when gid is a set of values such as [1,2,8,10]. The multple gid type must be set up as some kind of struct/class. For example: struct gids { int x[4]; }; It should work with a default constructor gids(). size_of(gids) should be valid (it can't contain mem allocs) It should be copyable/comparable, so a=b works if a and b are both gids type: Otherwise the content of the struct/class is arbitrary. This update removes the #ifdef macros which distinguished between the vector user_t mode and the single type mode. Now they can both work simultaneously and both are setup and tested in the unit test. Presently I am using two derived Directory classes to handle with/without variable vector user_t. Not clear if this will be the best API setup.
Adds test for performance evaluation. Updates the clock timing mechanisms. This is in progress. On the first pass the biggest hit was in remove due to the way I was handling the kokkos unorderedmap erase. This has now been fixed. Also made improvements to the initial guess size of the kokkos map which had a lot of impact on the performance. For larger scale problems the cost of the new kokkos mode is about twice that of the original code so there are still several places that need to be fixed.
This restores the ability to use NULL as an on/off flag and removes the bool I added to track that.
Now the lid works and is tested. There is an inefficiency in the new directory compared to the original where memory for lid is always allocated but this will be fixed in future updates to be more like the original zoltan directory. The lid can be a simple type or a struct type (same as gid) and both are tested and validated.
std::vector initialization of members was an issue for performance so refactored to use ArrayRCP. Also implemented some other performance changes and added a bunch of clocks. Some zoltan changes here are temporary for performance comparisons and not intended for the final version. (All clock code could eventually be scrapped).
Sets debug level back to 0 which I had accidentally left on for the last update.
A general set of updates to organize and clean things up better. Also fixes some issues. - Added much more extensive code comments - Finished refactoring copy constructor and operator= - Added unit test coverage for copy constructor and operator= - Added unit test coverage for calling update twice on same directory. - Fixed issue when calling find but ignoring user data. - Fixed issue when using lid type of larger size than gid type. - Changed vector size storage to be size_t. - Cleaned up various places for consistency or efficiency. - Includes some currently disabled in progress code for adding functors. - Improved some name spacing issues for consistency.
This adds an interface for generating unique Ids. Adds a new directory test Zoltan2_directory_findUniqueGids This test closely follows the original findUniqueGids test but uses the new interface for the directory instead of directly writing to the user data. This update also adds support so the directory tests can all run in Serial mode. This was primarily done for debugging. I also fixed some warnings I get in the original Zoltan2_findUniqueGids.hpp These come up for clang and I think this will work in all cases. There are some other similar warnings in Zoltan2 I did not fix yet.
Restores format to be like original zoltan directory to pass ptr and length instead of std::vector. This was discussed to prevent some performance situations where std::vector had to be generated to use the API.
Implements the new directory for Zoltan2_GraphMetricsUtility.hpp Combines all the methods into a single unified method. A new AggregateAdd option was added to the directory to facilitate combining structs. This currently does not assume any ordering but we could make that a requirement. The structs determine if they match through operator== and if they do operator+= is used to combine them. This design needs discussion. Adds some testing to Zoltan2_Metric for graphs and also some simple validation to verify the edge cuts are calculated as expected.
Updates TaskMapping to use the new directory. This should calculate identical results to the prior but now uses the new directory in all places.
Added graph metrics testing. Added additional checks so the test would fail if the graph metrics or imbalance metrics calculations are incorrect.
Removed Original, Relic and Tpetra modes. Removes Performance code for comparing them. Removes all macro definitions for choosing modes. Removes all clock code.
This had a mistake for serial and was getting a bit convoluted so pulled this recent addition out and will do something better. Fixes serial to pass again.
github issue #3727
Changed `import RCP` to `from . import RCP`
Origin repo remote tracking branch: 'github/master' Origin repo remote repo URL: 'github = [email protected]:TriBITSPub/TriBITS.git' At commit: commit adbf3c11f154d5b308fcc7e0188588892de6c488 Author: Roscoe A. Bartlett <[email protected]> Date: Fri Jan 11 16:07:37 2019 -0700 Summary: Add support for CMAKE_DO_INSTALL (#2689)
@trilinos/tpetra For #4143, add a constructor to BlockVector with the following prototype: BlockVector(const vec_type& X_vec, const map_type& meshMap, const LO blockSize); intended for use for a Thyra::createVector() function that takes a Tpetra::BlockMultiVector.
This new test is added on to the end of the BlockMultiVector ctor test. Note that it currently fails, pending debugging.
Changing test default to use RHS as initial vector for Hybrid Gmres #4159
…-warnings Automatically Merged using Trilinos Pull Request AutoTester PR Title: Remove Unused Parameter Warnings PR Author: jmgate
Tpetra - fix warning error from mismatched virtual functions
Add infrastructure for CUDA PR build
Switch Kokkos::unordered_map to std::unordered_map as further work needs to be done resolving types first. Copy murmur hash code in directly to remove link error for Albany. To remove/refactor later for Kokkos development. Disabled one new test that needs modification to work with unordered map.
Automatically Merged using Trilinos Pull Request AutoTester PR Title: Another check for Tpetra_DefaultNode. PR Author: searhein
Fix CUDA and Albany builds from prior PR #1533
Status Flag 'Pre-Test Inspection' - Auto Inspected - Inspection Is Not Necessary for this Pull Request. |
Status Flag 'Master Merge AutoTester' - Testing Jenkins Projects: Pull Request Auto Testing STARTING (click to expand)Build InformationTest Name: Trilinos_pullrequest_gcc_4.8.4
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_4.9.3_SERIAL
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_7.2.0
Jenkins Parameters
Using Repos:
Pull Request Author: trilinos-autotester |
Status Flag 'Master Merge AutoTester' - Jenkins Testing: all Jobs PASSED Pull Request Auto Testing has PASSED (click to expand)Build InformationTest Name: Trilinos_pullrequest_gcc_4.8.4
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_intel_17.0.1
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_4.9.3_SERIAL
Jenkins Parameters
Build InformationTest Name: Trilinos_pullrequest_gcc_7.2.0
Jenkins Parameters
|
Status Flag 'Pre-Merge Inspection' - Auto Inspected - Inspection Is Not Necessary for this Pull Request. |
Status Flag 'Master Merge AutoTester' - Pull Request MUST BE MERGED MANUALLY BY Project Team - Master Automerge is disabled (in .cfg file) |
2 similar comments
Status Flag 'Master Merge AutoTester' - Pull Request MUST BE MERGED MANUALLY BY Project Team - Master Automerge is disabled (in .cfg file) |
Status Flag 'Master Merge AutoTester' - Pull Request MUST BE MERGED MANUALLY BY Project Team - Master Automerge is disabled (in .cfg file) |
Auto PR created to promote from master_merge_20190124_010614 branch to master